Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid phantom values in NDK Metadata #1700

Merged
merged 1 commit into from
Jun 13, 2022
Merged

Conversation

lemnik
Copy link
Contributor

@lemnik lemnik commented Jun 10, 2022

Goal

Fix a bug when overwriting of clearing metadata, the NDK did not always remove the value as expected leading to phantom metadata values in reports. For example:

addMetadata("section", "overwrite", "value1")
addMetadata("section", "overwrite", "value2")
clearMetadata("section", "overwrite")

Would typically result in the Event having "value1" in its reported metadata, instead of no value as expected.

Design

Introduce a simple compacting function which will remove any duplicates and gaps in the metadata array and compact it.

This new function is used when either:

  • an entry or entries are removed from the metadata
  • the metadata array is full and a new value is being added

By retaining the insertion order of the metadata we introduce minimal additional overhead on the most common path: adding metadata. The correct values are retained as the most recent values are always the last in the array (effectively acting as a primitive journal).

Designs considered but not used

  • A linear scan to check for existing values in the add functions - this would have introduce unacceptable overheads
  • A full hash-map style implementation - requires a significant rewrite of the entire metadata model

Testing

Modified the existing end-to-end test which deletes metadata before a native crash

@lemnik lemnik requested a review from kstenerud June 10, 2022 12:42
@lemnik lemnik force-pushed the PLAT-8548/ndk-meta-overwrite branch from 646db2e to ab3e1e5 Compare June 10, 2022 12:43
@bugsnagbot
Copy link
Collaborator

bugsnagbot commented Jun 10, 2022

Android notifier sizes

Format Size impact of Bugsnag (kB) Size impact of Bugsnag when Minified (kB)
APK 1855.51 1606.17
arm64_v8a 651.66 401.8
armeabi_v7a 586.13 340.37
x86 725.37 475.51
x86_64 692.61 446.85

Generated by 🚫 Danger

Copy link
Contributor

@kstenerud kstenerud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugly as hell O^2 algo, but for so few entries it's the sane choice :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants